[Core] Fix the unnecessary logs #36931
Conversation
Signed-off-by: SangBin Cho <rkooo567@gmail.com>
| RAY_LOG(ERROR) << "Failed to get is_running_tasks from core worker: " | ||
| << status.ToString(); | ||
| RAY_LOG(WARNING) << "Failed to get is_running_tasks from core worker: " | ||
| << status.ToString(); |
There was a problem hiding this comment.
hmm, if the status is not ok, should we early return, seems the current code keeps executing.
There was a problem hiding this comment.
Let's not handle it in this PR and make a follow-up? This error message actually impacts the user experience, so let's make sure we merge this PR first. cc @architkulkarni to follow up with this comment (we probably want to fix that by 2.6 as well).
There was a problem hiding this comment.
@jjyao @rkooo567 In the case of returning early from status not ok for one of the jobs:
- should we set is_running_tasks to True or False?
- should we set the status in the overall reply (which contains info for all the jobs) to OK, or something else?
Also, why should it be a 2.6 release blocker? Will it crash if it doesn't return early?
There was a problem hiding this comment.
I think we should reply OK but have a special flag to indicate the result is not known.
There was a problem hiding this comment.
the ERROR log is a release blocker. This can easily happen under common cases, and ERROR logs will make drivers spammy. I already saw this message frequently from CI tests.
This error log could be very spammy.
This error log could be very spammy. Signed-off-by: e428265 <arvind.chandramouli@lmco.com>
Why are these changes needed?
This error log could be very spammy.
Related issue number
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/under thecorresponding
.rstfile.